Skip to content

Antalya 26.3 Add 'PRs in Release' table to report#1592

Open
strtgbb wants to merge 11 commits into
antalya-26.3from
report-merged-prs
Open

Antalya 26.3 Add 'PRs in Release' table to report#1592
strtgbb wants to merge 11 commits into
antalya-26.3from
report-merged-prs

Conversation

@strtgbb

@strtgbb strtgbb commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

@strtgbb strtgbb added cicd Improvements and fixes to the CICD process antalya-26.1 labels Mar 27, 2026
@github-actions

github-actions Bot commented Mar 27, 2026

Copy link
Copy Markdown

Workflow [PR], commit [62424d5]

@strtgbb

strtgbb commented Apr 1, 2026

Copy link
Copy Markdown
Collaborator Author

Should cicd PRs be included in the table?

@strtgbb strtgbb force-pushed the report-merged-prs branch from 441c6d9 to bceb0a1 Compare April 8, 2026 16:03
@strtgbb strtgbb force-pushed the report-merged-prs branch from bceb0a1 to b5c51a3 Compare May 14, 2026 13:48
@strtgbb strtgbb changed the base branch from antalya-26.1 to antalya-26.3 May 14, 2026 13:48
@strtgbb strtgbb changed the title Antalya 26.1 Add 'PRs in Release' table to report Antalya 26.3 Add 'PRs in Release' table to report May 14, 2026
@strtgbb strtgbb force-pushed the report-merged-prs branch from b5c51a3 to 09082a7 Compare May 21, 2026 18:25
@strtgbb strtgbb force-pushed the report-merged-prs branch 2 times, most recently from 3174cba to 567fe40 Compare June 11, 2026 19:20
@strtgbb strtgbb force-pushed the report-merged-prs branch from 567fe40 to e853348 Compare June 15, 2026 18:06
@CarlosFelipeOR

CarlosFelipeOR commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

AI audit note: This review comment was generated by AI (gpt-5.3-codex).

Audit update for PR #1592 (add PRs in Release table + tabbed report UI):

Confirmed defects:

High: Stored HTML/script injection in PRs in Release report table
Impact: A merged PR title/label containing HTML can execute in the generated CI report page when maintainers open it.
Anchor: .github/actions/create_workflow_report/create_workflow_report.py / _enrich_prs_in_release_merge_prs, format_pr_labels_with_verification, format_results_as_html_table
Trigger: Any merged PR with title or label text like <img src=x onerror=alert(1)>.
Why defect: Untrusted PR metadata is inserted into table cells and rendered with escape=False; label text is also interpolated into raw HTML.
Fix direction (short): Escape pr_name/pr_labels (or use escape=True and only allow explicit safe HTML columns).
Regression test direction (short): Unit-test malicious PR title/label and assert rendered output is escaped text, not executable markup.

Low: Error-path fallback can render raw Python list ([]) instead of user-facing message
Impact: On recoverable data-collection failures, report quality degrades to confusing literal output instead of an explicit “nothing to report/error” state.
Anchor: .github/actions/create_workflow_report/create_workflow_report.py / create_workflow_report, format_results_as_html_table
Trigger: get_prs_in_release_dataframe exception on release report path.
Why defect: The exception path leaves prs_in_release as [], and formatter now returns non-DataFrame values unchanged.
Fix direction (short): Keep prs_in_release as an empty DataFrame or make formatter normalize non-DataFrames to a safe fallback HTML message.
Regression test direction (short): Simulate get_prs_in_release_dataframe failure and assert output does not contain literal [].

Coverage summary:

  • Scope reviewed: full changed call graph in 2 files (create_workflow_report.py, ci_run_report.html.jinja), including release-only path, rendering path, and tab-navigation path.
  • Categories failed: untrusted-data output encoding, error-path rendering contract.
  • Categories passed: normal tab switching flow, PR-vs-release gating, preview gating, git/release-baseline fallback containment (exceptions are caught in report generation path).
  • Assumptions/limits: static reasoning only (no live GH Actions/browser execution in this run); no C++/multithreaded/shared-state paths in scope.

…jection

Signed-off-by: CarlosFelipeOR <carlosfelipeor@gmail.com>
@CarlosFelipeOR

Copy link
Copy Markdown
Collaborator

Fixed the High defect (HTML-escape of PR title and labels):
62424d5

The Low defect isn't exclusive to the new table: the early-return
if not isinstance(results, pd.DataFrame): return results in
format_results_as_html_table makes any empty-list fallback render
as literal []. Since it touches shared logic, can be addressed in
a separate PR if needed.

@CarlosFelipeOR

Copy link
Copy Markdown
Collaborator

QA Verification

Verdict: passed, with one open design question (see below).

This PR only changes the CI workflow report generator (Python + Jinja), so test suites were not considered. Verification was visual + functional on the rendered HTML reports.

Reports inspected:

1. Tabbed navigation replaces the old TOC + <h2> sections

  • Tab strip renders at the top with the correct buttons and counters.
  • Only one tab content is visible at a time; switching tabs swaps content correctly and marks the active tab.
  • URL #hash is updated on click and deep-linking (…#regression-fails, …#checks-errors, etc.) opens directly in that tab on reload.
  • Invalid hash falls back to CI Jobs Status, as intended.
  • Tab strip wraps correctly on narrow widths; no console errors.

2. New "PRs in Release" tab (branch reports only)

  • In the PR-mode report (pr_number != 0), the tab is correctly absent.
  • In the branch report (pr_number == 0), the tab is present as the first tab with the correct counter (95).
  • Columns render as PR Name, PR Number, PR Labels (header correctly shows PR in uppercase).
  • PR Number values are clickable links to the corresponding GitHub PR.
  • Titles and labels match the actual PRs on GitHub on the samples I cross-checked.
  • In preview mode, the placeholder `"PR details are not loaded during preview."` is shown instead of the table, as expected.

3. `(missing verification)` highlighting

  • After removing the `verified` label from one of the PRs and regenerating the report, that PR's `PR Labels` cell renders in bold red with the `(missing verification)` suffix appended, exactly as implemented in `format_pr_labels_with_verification`.
  • All other PRs (still carrying `verified`) render with normal styling.

4. CSS / visual changes

  • Row hover uses the new softer `--altinity-hover` tone instead of light gray.
  • Link hover color (`#FFC600`) preserved via renamed `--altinity-link-hover`.
  • Tables now expand to 100% width on narrower viewports; horizontal scroll happens inside `.tabcontent` when needed.
  • Preview banner ("This is a preview…") still renders above the tab strip when applicable.
  • Header date is rendered in UTC and matches the run time (consistent with the `datetime.now(timezone.utc)` change).

5. Sections that previously had inline text are preserved inside their tabs

  • "Checks Known Fails" still shows the KNOWN/INVESTIGATE/NEEDSFIX conventions paragraph inside the tab.
  • "New Fails in PR" still shows the `Compared with base sha …` line inside the tab.

Open question / suggestion

On a branch (release) report, the default tab is still CI Jobs Status, even when PRs in Release is present as the first tab. Is this intentional? For a release report it would arguably make more sense to land directly on "PRs in Release", which is the new feature surfaced by this PR — otherwise the new tab is somewhat hidden behind a click. Happy to keep current behavior if it was a deliberate consistency choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya antalya-26.1 antalya-26.3 cicd Improvements and fixes to the CICD process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants